Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resize the browser to match VNC session #1849

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

XiaoXianNv-boot
Copy link

I've added a new zoom mode that allows the browser to automatically adapt to the window .
Just need to use window.open('vnc. html/?token=token&autoconnect=1&resize=browser','_blank','toolbar=no,location=no,status=no,menubar=no,resizable=yes,width=800,height=420'); Open a window
Works fine on EDGE

@CendioOssman
Copy link
Member

Thank you for your contribution!

I'm thinking this might be a bit too niche, though, since it only works under very specific circumstances. Could you describe your setup a bit more, and where this feature fits in?

@XiaoXianNv-boot
Copy link
Author

Used to open the QEMU/KVM console in a new browser window, yes, QEMU/KVM will constantly change the resolution when it is launched

@XiaoXianNv-boot
Copy link
Author

I'm connected to the VNC port of KVM/QEMU via websockify

@XiaoXianNv-boot
Copy link
Author

Copy link
Member

@CendioOssman CendioOssman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. That sounds reasonble. I'm cautious about adding niche stuff to the general UI, though.

In TigerVNC, we have a similar behaviour but without a setting. Perhaps that could be used here as well?

The principle there is that it will resize the local window if the sizes match before the resize, but not if they are different. I.e.

  1. VNC session is 1024x768, local window is 1024x768. VNC session resizes to 1920x1080, and the local window tries to resize to the same 1920x1080.
  2. VNC session is 1024x768, but local window is 800x600. VNC session resizes to 1920x1080, but the local window stays at 800x600.

@samhed samhed added the feature label May 17, 2024
@CendioOssman
Copy link
Member

@XiaoXianNv-boot, are you interested in looking at the suggested changes?

@XiaoXianNv-boot
Copy link
Author

@XiaoXianNv-boot, are you interested in looking at the suggested changes?

YES

@XiaoXianNv-boot
Copy link
Author

Okay. That sounds reasonble. I'm cautious about adding niche stuff to the general UI, though.

In TigerVNC, we have a similar behaviour but without a setting. Perhaps that could be used here as well?

The principle there is that it will resize the local window if the sizes match before the resize, but not if they are different. I.e.

  1. VNC session is 1024x768, local window is 1024x768. VNC session resizes to 1920x1080, and the local window tries to resize to the same 1920x1080.
  2. VNC session is 1024x768, but local window is 800x600. VNC session resizes to 1920x1080, but the local window stays at 800x600.

I have added adjustment rules now. Meet automatic adjustment in the following circumstances

  1. The first adjustment size after setting this mode
  2. The size of the current browser window is the size of the previous adjustment

If the above requirements are not met, the size of the browser will be disabled.
After adjusting to other modes, you can re -enable this mode (View Rules 1)

Copy link
Member

@CendioOssman CendioOssman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a good step in the right direction. But I think some more adjustments are needed to get a good fit.

Please have a look and see the comments.

vnc.html Outdated
@@ -171,6 +171,7 @@ <h1 class="noVNC_logo" translate="no"><span>no</span><br>VNC</h1>
<option value="off">None</option>
<option value="scale">Local Scaling</option>
<option value="remote">Remote Resizing</option>
<option value="browser" disabled="" id="noVNC_setting_browser_resize">Resizing browser</option>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need a new setting for this. This should be possible to have active as long as things aren't configured to "scale".

core/rfb.js Outdated
@@ -2863,10 +2879,36 @@ export default class RFB extends EventTargetMixin {
this._fbWidth, this._fbHeight);
}

_updateBrowserWindows(width, height) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The RFB object isn't really aware of things outside of its container. So I think these things are better placed in the UI object.

That means it needs some more information, though. We require width and height read-only properties on the RFB object. And we also need a resize event to be fired.

core/rfb.js Outdated
if (bodyWidthBrowserResize === document.body.clientWidth &&
bodyHeightBrowserResize === document.body.clientHeight) {
OldResolutionEqual = true;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this gives the desired behaviour. What we want to compare with is _fbWidth and _fbHeight just before the resize, and see if they match the browser window.

This code might still be doing that, but it's not obvious that it does.

@samhed samhed changed the title New zoom mode to resize the browser Resize the browser to match VNC session Aug 19, 2024
@CendioOssman
Copy link
Member

It looks like this was closed accidentally, @XiaoXianNv-boot?

@XiaoXianNv-boot
Copy link
Author

看起来这是意外关闭的,?

YES

@XiaoXianNv-boot
Copy link
Author

What is the situation

set resizeBrowser(void_) {
this._resizeBrowser = void_;
if (this._resizeBrowser && (this._rfbConnectionState === 'connected')) {
this._resizeBrowser(this._fbWidth, this._fbHeight);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Callbacks are not that flexible. Please use a CustomEvent like we do for other things in the RFB object.

@@ -2875,6 +2884,10 @@ export default class RFB extends EventTargetMixin {
this._fbWidth = width;
this._fbHeight = height;

if (this._resizeBrowser) {
this._resizeBrowser(width, height);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably best if this happens last, so that all state is properly updated.

<html lang="en">
<head>
<title>noVNC</title>
</head>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although an example is useful, I don't think we want another HTML file to maintain. I think it is sufficient with some information in the documentation somewhere.

@@ -1058,6 +1084,12 @@ const UI = {
UI.rfb.clipViewport = UI.getSetting('view_clip');
UI.rfb.scaleViewport = UI.getSetting('resize') === 'scale';
UI.rfb.resizeSession = UI.getSetting('resize') === 'remote';
if (UI.getSetting('resize') === 'off') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be relevant for 'remote' as well, I think?

UI.rfb.resizeBrowser = UI._updateBrowserWindows;
} else {
UI.rfb.resizeBrowser = false;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be simpler to always have the callback registered, and check the setting once the callback fires?


let OldResolutionEqual = false;
if (UI.bodyWidthBrowserResize === document.body.clientWidth &&
UI.bodyHeightBrowserResize === document.body.clientHeight) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this is a very roundabout way of checking things. Why not compare with the previous VNC session size?

if (UI.bodyHeightBrowserResize === 0 ||
OldResolutionEqual) {
if ((width != 0) && (height != 0)) {
window.resizeBy(width - bodyWidth, height - bodyHeight);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not resizeTo() and avoid having to calculate things?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants